Skip to content

security: redact CLI logs, bound regex matching (ReDoS), validate Chromium base URL (PER-8609/8615/8616)#2279

Open
Shivanshu-07 wants to merge 1 commit into
masterfrom
security/cli-runtime-redact-redos-ssrf
Open

security: redact CLI logs, bound regex matching (ReDoS), validate Chromium base URL (PER-8609/8615/8616)#2279
Shivanshu-07 wants to merge 1 commit into
masterfrom
security/cli-runtime-redact-redos-ssrf

Conversation

@Shivanshu-07

@Shivanshu-07 Shivanshu-07 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Second focused percy-cli security PR — three contained runtime findings (deadline 2026-06-16) in @percy/core.

Ticket CWE Finding
PER-8609 CWE-532 CLI logs transmitted to Percy API without secret redaction
PER-8615 CWE-1333 ReDoS via user-controlled regex in snapshot include/exclude
PER-8616 CWE-918 SSRF via unvalidated PERCY_CHROMIUM_BASE_URL download target

Changes

percy.js (PER-8609): sendBuildLogs() sent clilogs raw while cilogs were already passed through redactSecrets(). Wrap clilogs in the same redactSecrets() so tokens / credential-bearing URLs are stripped before egress.

snapshot.js (PER-8615): snapshotMatches() runs user-controllable regex/glob patterns against snapshot.name. A crafted long name reaching the matcher (e.g. via the local API — chain PER-8627) plus a backtracking-prone pattern could hang the process. Added MAX_MATCH_INPUT_LENGTH = 2048: glob/RegExp matching is skipped for over-long names (exact-string matching is unaffected, and real snapshot names are short).

install.js (PER-8616): Added resolveChromiumBaseUrl()PERCY_CHROMIUM_BASE_URL must be a well-formed HTTPS URL, otherwise it warns and falls back to the trusted default host. Private HTTPS mirrors remain supported (the doctor check documents them), so no host allowlist.

Verification (against real source)

  • resolveChromiumBaseUrl: https mirror kept; http://, ftp://, and malformed values rejected → default. ✅
  • redactSecrets on the clilogs array shape: GitHub token + bearer redacted to [REDACTED]. ✅
  • ReDoS guard: catastrophic (a+)+$ against a 50k-char input returns in 0 ms (would otherwise hang); normal patterns still match. ✅
  • Existing sendBuildLogs tests use secret-free messages (redaction is a no-op → content matches); the Chromium with custom URL test uses an HTTPS URL (compatible).

PER-8605 (Chromium binary integrity) is partially addressed here — HTTPS is now enforced, giving transport integrity from the trusted host. Full checksum pinning of the downloaded binary (per platform/revision) is a separate follow-up since it needs canonical hashes for all 5 platform builds.

Closes PER-8609, PER-8615, PER-8616. Partially addresses PER-8605; mitigates the ReDoS leg of PER-8627.

🤖 Generated with Claude Code

…omium base URL (PER-8609/8615/8616)

Three contained runtime hardening fixes in @percy/core:

PER-8609 (CWE-532) — clilogs were sent to the Percy API without secret
redaction (cilogs already were). Wrap clilogs in the existing redactSecrets()
so tokens / credential-bearing URLs are stripped before egress.

PER-8615 (CWE-1333) — snapshotMatches() runs user-controllable regex/glob
patterns against snapshot.name; a crafted long name reaching the matcher
(e.g. via the local API, per chain PER-8627) could trigger catastrophic
backtracking. Cap the matched-input length (MAX_MATCH_INPUT_LENGTH = 2048)
before any RegExp/micromatch call; exact-string matching is unaffected.

PER-8616 (CWE-918) — PERCY_CHROMIUM_BASE_URL was used as a download base with
no validation, enabling SSRF / an integrity downgrade. Add
resolveChromiumBaseUrl(): require a well-formed HTTPS URL, otherwise warn and
fall back to the trusted default host. (Private HTTPS mirrors remain supported,
so no host allowlist; this also gives transport integrity for PER-8605 — full
checksum pinning of the binary is a separate follow-up.)

Verified against real source: resolveChromiumBaseUrl (https-only + fallback),
redactSecrets on the clilogs array shape (GitHub token + bearer redacted), and
the ReDoS guard (catastrophic pattern on a 50k-char input returns in 0ms).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Shivanshu-07 Shivanshu-07 requested a review from a team as a code owner June 14, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant